Skip to content

Conversation

@thierry-f-78
Copy link

Fix: Stack Overflow in table.concat Function

This PR fixes a stack overflow issue in the table.concat function when working with large tables.

Problem

The previous implementation of tableConcat pushed all values and separators onto the Lua stack before concatenating them. This approach caused stack overflow errors when dealing with large tables.

Solution

This fix changes the implementation to build the result string incrementally rather than pushing all values onto the stack first. Instead of accumulating items on the stack, we now:

  • Build the string directly by appending each value
  • Add separators between values as needed
  • Push only the final result string to the stack

Testing

The fix has been verified with large tables that previously caused stack overflow. For example, this test case now works correctly:

package main

import "github.com/yuin/gopher-lua"

func main() {
        var L *lua.LState
        var err error

        L = lua.NewState()
        defer L.Close()

        err = L.DoString(`
                local t = {}
                for i = 1, 10000 do
                        t[i] = tostring(i)
                end
                local s = table.concat(t, ',')
        `)
        if err != nil {
                println(err.Error())
        }
}

This example would previously fail with a stack overflow error but now executes successfully.

tablelib.go Outdated
}
}
L.Push(stringConcat(L, L.GetTop()-retbottom, L.reg.Top()-1))
L.Push(LString(result))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth considering go lang string builder? https://pkg.go.dev/strings#Builder ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply, I missed the notification!

Thanks for the suggestion! Indeed, strings.Builder would be more efficient for string construction, especially with large tables. I'll update the code to use strings.Builder instead of simple concatenation.

The previous implementation of tableConcat was pushing all values and
separators onto the Lua stack before concatenating them, which caused
stack overflow with large tables.

This fix changes the implementation to build the result string
incrementally rather than pushing all values onto the stack first,
preventing stack overflow with large tables.

Example that would previously fail but now works correctly:
```
package main

import "github.com/yuin/gopher-lua"

func main() {
	var L *lua.LState
	var err error

	L = lua.NewState()
	defer L.Close()

	err = L.DoString(`
		local t = {}
		for i = 1, 10000 do
			t[i] = tostring(i)
		end
		local s = table.concat(t, ',')
	`)
	if err != nil {
		println(err.Error())
	}
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants